Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

opaque_closure: Lookup optimized oc source inside code instance #53878

Merged
merged 1 commit into from
Mar 27, 2024

Conversation

Keno
Copy link
Member

@Keno Keno commented Mar 27, 2024

This is an alternative to #53852. I don't think it's semantically legal to put optimized source into the :source field of a method, but it should be fine to just look it up from the CodeInstance. That said, this is a bit of an unusual configuration. In particular it wasn't even reachable with the surface APIs, which assumed that inferred IR was always supposed to be compiled.

Copy link
Sponsor Member

@aviatesk aviatesk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that our system can interpret optimized source, why isn't it allowed to put optimized code into method->source?

@Keno
Copy link
Member Author

Keno commented Mar 27, 2024

It's not in general legal to re-run inference on it, but putting it in method->source might suggest it would be. Also, the validity is world age dependent, while method->source should not be. Of course opaque closure semantics make it more confusing, but I think this is better.

Copy link
Sponsor Member

@aviatesk aviatesk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I understand. This approach is better than #53852 .

This is an alternative to #53852. I don't think it's semantically legal
to put optimized source into the :source field of a method, but it
should be fine to just look it up from the CodeInstance. That said,
this is a bit of an unusual configuration. In particular it wasn't
even reachable with the surface APIs, which assumed that inferred
IR was always supposed to be compiled.
@Keno Keno merged commit 4ee1022 into master Mar 27, 2024
5 of 7 checks passed
@Keno Keno deleted the kf/ocoptsource2 branch March 27, 2024 19:44
Keno added a commit that referenced this pull request Apr 2, 2024
aviatesk pushed a commit that referenced this pull request Apr 3, 2024
aviatesk added a commit that referenced this pull request Apr 3, 2024
Reflection version of #53878.

---------

Co-authored-by: Shuhei Kadowaki <[email protected]>
Co-authored-by: Shuhei Kadowaki <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants